-
Notifications
You must be signed in to change notification settings - Fork 2
Add selfie support #124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add selfie support #124
Conversation
WalkthroughThe Verification class now treats steps with id 'selfie' the same as 'liveness': proof_of_life_document returns the last step whose id is in ['liveness', 'selfie'], proof_of_life_errors aggregates errors from both step types, and a new public property proof_of_life_url returns video_url or selfie_photo_url from that step. The package version in mati/version.py was bumped from '2.0.7' to '2.0.8'. Tests were updated to assert that verification.proof_of_life_url is truthy for a full verification and falsy when no liveness/selfie step exists. Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
mati/resources/verifications.py (1)
93-105: Include selfie errors and simplify error dict accessTo align with the new selfie support in proof_of_life_document, also surface selfie errors here. While at it, avoid repeated
'in'checks by using.get(...).def proof_of_life_errors(self) -> List[Errors]: - return [ - Errors( - identifier=pol.id, - type=pol.error['type'] if 'type' in pol.error else None, - code=pol.error['code'] if 'code' in pol.error else None, - message=pol.error['message'] - if 'message' in pol.error - else None, - ) - for pol in self.steps # type: ignore - if pol.id == 'liveness' and pol.error - ] + return [ + Errors( + identifier=pol.id, + type=pol.error.get('type'), + code=pol.error.get('code'), + message=pol.error.get('message'), + ) + for pol in (self.steps or []) # type: ignore + if pol.id in ('liveness', 'selfie') and pol.error + ]
🧹 Nitpick comments (1)
mati/resources/verifications.py (1)
85-89: Use membership test for brevity and readabilitySimplify the condition to use an in-check.
- pol = [ - pol - for pol in self.steps - if (pol.id == 'liveness' or pol.id == 'selfie') - ] + pol = [pol for pol in self.steps if pol.id in ('liveness', 'selfie')]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
mati/resources/verifications.py(1 hunks)mati/version.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit Configuration File
**/*.py: Enforce Relative Imports for Internal ModulesEnsure that any imports referencing internal modules use relative paths. However, if modules reside in the main module directories (for example /src or /library_or_app_name) —and relative imports are not feasible—absolute imports are acceptable. Additionally, if a module is located outside the main module structure (for example, in /tests or /scripts at a similar level), absolute imports are also valid.
Examples and Guidelines:
- If a module is in the same folder or a subfolder of the current file, use relative imports. For instance: from .some_module import SomeClass
- If the module is located under /src or /library_or_app_name and cannot be imported relatively, absolute imports are allowed (e.g., from library_or_app_name.utilities import helper_method).
- If a module is outside the main module directories (for example, in /tests, /scripts, or any similarly placed directory), absolute imports are valid.
- External (third-party) libraries should be imported absolutely (e.g., import requests).
**/*.py:
Rule: Enforce Snake Case in Python Backend
- New or Modified Code: Use snake_case for all variables, functions, methods, and class attributes.
- Exceptions (Pydantic models for API responses):
- Primary fields must be snake_case.
- If older clients expect camelCase, create a computed or alias field that references the snake_case field.
- Mark any camelCase fields as deprecated or transitional.
Examples
Invalid:
class CardConfiguration(BaseModel): title: str subTitle: str # ❌ Modified or new field in camelCaseValid:
class CardConfiguration(BaseModel): title: str subtitle: str # ✅ snake_case for new/modified field @computed_field def subTitle(self) -> str: # camelCase allowed only for compatibility return self.subtitleAny direct use of camelCase in new or updated code outside of these exceptions should be flagged.
`*...
Files:
mati/resources/verifications.pymati/version.py
🔇 Additional comments (2)
mati/resources/verifications.py (1)
82-90: No changes needed: selfie steps use Liveness
Since__post_init__maps every entry inself.stepsthroughLiveness._from_dict, both"liveness"and"selfie"IDs becomeLivenessinstances withstatus,error, etc. The return typeOptional[Liveness]is therefore accurate.mati/version.py (1)
1-1: Version bump looks goodVersion updated to 2.0.8; aligns with the new selfie support change.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #124 +/- ##
==========================================
+ Coverage 98.13% 98.16% +0.03%
==========================================
Files 12 12
Lines 375 382 +7
==========================================
+ Hits 368 375 +7
Misses 7 7
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
mati/resources/verifications.py (1)
95-108: Refactor to EAFP for nested dict access and handle None steps.Aligns with the coding guideline to use try/except for nested dict keys and avoids iterating over None. Also reuse the shared step IDs constant.
Apply this diff:
@property def proof_of_life_errors(self) -> List[Errors]: - return [ - Errors( - identifier=pol.id, - type=pol.error['type'] if 'type' in pol.error else None, - code=pol.error['code'] if 'code' in pol.error else None, - message=pol.error['message'] - if 'message' in pol.error - else None, - ) - for pol in self.steps # type: ignore - if pol.id in ['liveness', 'selfie'] and pol.error - ] + if not self.steps: + return [] + errors: List[Errors] = [] + for step in self.steps: + if step.id in self._POL_STEP_IDS and step.error: + try: + err_type = step.error['type'] + except (KeyError, TypeError): + err_type = None + try: + code = step.error['code'] + except (KeyError, TypeError): + code = None + try: + message = step.error['message'] + except (KeyError, TypeError): + message = None + errors.append( + Errors( + identifier=step.id, + type=err_type, + code=code, + message=message, + ) + ) + return errors
♻️ Duplicate comments (1)
mati/resources/verifications.py (1)
95-108: Including 'selfie' in proof_of_life_errors addresses the prior review.Good catch extending the filter to include selfie steps.
🧹 Nitpick comments (2)
mati/resources/verifications.py (1)
85-86: Avoid magic strings; extract step IDs constant and fix shadowing for readability.Prevents duplication across methods and avoids confusing reuse of the variable name "pol". Use a shared constant and a clearer local name.
Apply this diff:
- pol = [pol for pol in self.steps if pol.id in ['liveness', 'selfie']] - return pol[-1] if pol else None + life_steps = [step for step in self.steps if step.id in self._POL_STEP_IDS] + return life_steps[-1] if life_steps else NoneAdd this class constant (outside the selected range, near the top of the class):
# Inside class Verification, after _endpoint _POL_STEP_IDS: ClassVar[tuple] = ('liveness', 'selfie')tests/resources/test_verifications.py (1)
24-24: LGTM; add a complementary negative case to harden coverage.Consider asserting that proof_of_life_url is falsy when steps are absent (empty and None) to prevent regressions.
Add these assertions in existing tests (outside the selected range):
# In test_verification_without_liveness verification.steps = [] assert not verification.proof_of_life_url # In test_verification_without_pol verification.steps = None assert not verification.proof_of_life_urlI can push a patch if you prefer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
mati/resources/verifications.py(2 hunks)mati/version.py(1 hunks)tests/resources/test_verifications.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- mati/version.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit Configuration File
**/*.py: Enforce Relative Imports for Internal ModulesEnsure that any imports referencing internal modules use relative paths. However, if modules reside in the main module directories (for example /src or /library_or_app_name) —and relative imports are not feasible—absolute imports are acceptable. Additionally, if a module is located outside the main module structure (for example, in /tests or /scripts at a similar level), absolute imports are also valid.
Examples and Guidelines:
- If a module is in the same folder or a subfolder of the current file, use relative imports. For instance: from .some_module import SomeClass
- If the module is located under /src or /library_or_app_name and cannot be imported relatively, absolute imports are allowed (e.g., from library_or_app_name.utilities import helper_method).
- If a module is outside the main module directories (for example, in /tests, /scripts, or any similarly placed directory), absolute imports are valid.
- External (third-party) libraries should be imported absolutely (e.g., import requests).
**/*.py:
Rule: Enforce Snake Case in Python Backend
- New or Modified Code: Use snake_case for all variables, functions, methods, and class attributes.
- Exceptions (Pydantic models for API responses):
- Primary fields must be snake_case.
- If older clients expect camelCase, create a computed or alias field that references the snake_case field.
- Mark any camelCase fields as deprecated or transitional.
Examples
Invalid:
class CardConfiguration(BaseModel): title: str subTitle: str # ❌ Modified or new field in camelCaseValid:
class CardConfiguration(BaseModel): title: str subtitle: str # ✅ snake_case for new/modified field @computed_field def subTitle(self) -> str: # camelCase allowed only for compatibility return self.subtitleAny direct use of camelCase in new or updated code outside of these exceptions should be flagged.
`*...
Files:
tests/resources/test_verifications.pymati/resources/verifications.py
🧬 Code Graph Analysis (1)
tests/resources/test_verifications.py (2)
tests/conftest.py (1)
verification(217-222)mati/resources/verifications.py (1)
proof_of_life_url(89-93)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
mati/version.py (1)
1-1: Optional: add a type hint for version to aid tooling.Minor improvement for editor/type-checker friendliness.
-__version__ = '2.0.8' +__version__: str = '2.0.8'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
mati/version.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit Configuration File
**/*.py: Enforce Relative Imports for Internal ModulesEnsure that any imports referencing internal modules use relative paths. However, if modules reside in the main module directories (for example /src or /library_or_app_name) —and relative imports are not feasible—absolute imports are acceptable. Additionally, if a module is located outside the main module structure (for example, in /tests or /scripts at a similar level), absolute imports are also valid.
Examples and Guidelines:
- If a module is in the same folder or a subfolder of the current file, use relative imports. For instance: from .some_module import SomeClass
- If the module is located under /src or /library_or_app_name and cannot be imported relatively, absolute imports are allowed (e.g., from library_or_app_name.utilities import helper_method).
- If a module is outside the main module directories (for example, in /tests, /scripts, or any similarly placed directory), absolute imports are valid.
- External (third-party) libraries should be imported absolutely (e.g., import requests).
**/*.py:
Rule: Enforce Snake Case in Python Backend
- New or Modified Code: Use snake_case for all variables, functions, methods, and class attributes.
- Exceptions (Pydantic models for API responses):
- Primary fields must be snake_case.
- If older clients expect camelCase, create a computed or alias field that references the snake_case field.
- Mark any camelCase fields as deprecated or transitional.
Examples
Invalid:
class CardConfiguration(BaseModel): title: str subTitle: str # ❌ Modified or new field in camelCaseValid:
class CardConfiguration(BaseModel): title: str subtitle: str # ✅ snake_case for new/modified field @computed_field def subTitle(self) -> str: # camelCase allowed only for compatibility return self.subtitleAny direct use of camelCase in new or updated code outside of these exceptions should be flagged.
`*...
Files:
mati/version.py
🔇 Additional comments (2)
mati/version.py (2)
1-1: LGTM: Version bumped to 2.0.8 (PEP 440-compliant).Patch bump aligns with the PR scope. No functional concerns in this file.
1-1: Single source of truth confirmed ✅
__version__ = '2.0.8'is defined only inmati/version.py.setup.pyloads that value viaSourceFileLoaderand uses it insetup(version=…).- No other hard-coded
version =entries found insetup.cfgor (nonexistent)pyproject.toml.__version__is re-exported inmati/__init__.py.No further changes needed.
Summary by CodeRabbit